Skip to content

Conversation

@bcrant
Copy link

@bcrant bcrant commented Nov 12, 2025

What does this PR do?

On final step of CI/CD, applying plan to prod, exit early (raise an error) before applying plan to prod if the PR merge state status is "blocked".

Why?
It is currently possible for synchronized deployments to get into a bad state, where the changes in a PR have been applied to the production environment but have not been merged (desychronized), with the following configurations:

  • Sychronized deployment
  • CI/CD Bot configured to use Deploy Command Signal
  • CI/CD Bot is NOT configured to use Required Approval Signal
  • Repository has Github CODEOWNERS branch protection rules

This is currently possible because the GithubController.deploy_to_prod() checks explicitly checks for "is_dirty" state, but if the only thing missing is CODEOWNER approval, the merge state status "is_blocked", not "is_dirty".
https://docs.github.com/en/enterprise-cloud@latest/graphql/reference/enums#mergestatestatus

@CLAassistant
Copy link

CLAassistant commented Nov 12, 2025

CLA assistant check
All committers have signed the CLA.

Comment on lines 777 to 780
raise CICDBotError(
"Merge commit cannot be cleanly created. Likely missing CODEOWNERS approval. "
"Please check PR and resolve any issues."
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things:

  1. The merge commit itself can be created so I don't think the first sentence is needed.
  2. CODEOWNERS approval is one way a PR can be blocked. Can you generalize this and say that a branch protection/ruleset requirement is likely not satisfied?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed! Will check back in morning and make sure all ci/cd passed

Copy link
Contributor

@eakmanrq eakmanrq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @bcrant for contributing this! One small comment and then we can get it in.

…issing CODEOWNERS approval, this is breaking synchronous deployments.
@bcrant bcrant force-pushed the bcrant/cicd-exit-before-deploy branch from 9688d27 to 2665c71 Compare December 3, 2025 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants